Skip to content

fix(activity): stuck confirmation on back after tx + missing lending deposit token icon cp-8.3.0#33157

Merged
vinnyhoward merged 10 commits into
mainfrom
fix-tmcu-1001-activity-confirmation-back-navigation
Jul 14, 2026
Merged

fix(activity): stuck confirmation on back after tx + missing lending deposit token icon cp-8.3.0#33157
vinnyhoward merged 10 commits into
mainfrom
fix-tmcu-1001-activity-confirmation-back-navigation

Conversation

@vinnyhoward

@vinnyhoward vinnyhoward commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Description

Two related Activity-redesign bug fixes on one branch.

1. Stuck confirmation on back after a transaction (TMCU-1001).
After submitting a full-screen confirmation (send, Earn pooled-staking, or stablecoin lending), the app redirects to the Activity screen. With the Money-account feature enabled, Activity is registered as a root-level screen, so the redirect pushed it on top of the still-mounted confirmation — whose approval had already been consumed (deleteAfterResult). Pressing back returned to that consumed confirmation, which rendered an infinite loader/skeleton.

The fix adds a shared navigateToActivityAfterConfirmation helper that replaces the confirmation's stack with Activity in a single StackActions.replace, removing the stale confirmation from the back stack. It's wired into the shared transaction confirm hook (useTransactionConfirm), batch approvals (useConfirmActions), the Earn lending deposit/withdrawal views, and the legacy staking footer.

Approach note : I first tried to make "back" return to the transaction build screen (amount / recipient / input). On a native-stack that means removing the nested confirmation while pushing a root-level Activity — two screen changes that react-native-screens animates at the same time, producing a double-navigation on the way in and a flashing half-finished pop on the way back. After several attempts to work around the animation, the reliable single-transition solution was to replace the confirmation's whole stack. As a result: send/staking → back returns to Wallet home; lending → back returns to the input screen (its input lives in a separate stack that survives the replace).

2. Missing token icon/amount on lending deposit rows (TMCU-1082).
Lending deposit Activity rows derived their token from the pool contract (txParams.to) instead of the underlying deposited asset, so the row had no resolvable icon, symbol, or amount. The supplied token is now resolved from simulation data / the outgoing Transfer log (falling back to the pool address), and lending token metadata is enriched from the tokens API; the row avatar uses the resolved token.

Changelog

CHANGELOG entry: Fixed the Activity screen back button returning to a stuck confirmation screen after a transaction, and fixed the missing token icon and amount on Earn lending deposit rows.

Related issues

Fixes: TMCU-1001
Fixes: TMCU-1082

Manual testing steps

Feature: Activity navigation after a transaction

  Background:
    Given the Money-account and Activity-redesign feature flags are enabled

  Scenario: Back after a send or staking transaction is not stuck
    Given I complete a native/ERC-20/NFT send, or an ETH pooled-staking deposit/unstake/claim
    When I am redirected to the Activity screen and tap the back button
    Then I return to Wallet home
    And I never land on the consumed confirmation screen or an infinite loader

  Scenario: Back after a lending deposit returns to the input screen
    Given I complete a stablecoin (USDC/USDT/DAI) lending deposit or withdrawal
    When I am redirected to the Activity screen and tap the back button
    Then I return to the lending input screen
    And I never land on the consumed confirmation screen

Feature: Lending deposit Activity row

  Scenario: Lending deposit row shows the deposited token
    Given I complete a stablecoin lending deposit
    When I open the Activity list
    Then the lending deposit row shows the underlying token's icon, symbol, and amount

Also covered by automated tests: real-navigator integration tests for all three navigation flows (navigateToActivityAfterConfirmation.test.tsx), the redirect call sites, and the lending row/adapter changes (ActivityListItemRow, local-transaction).

Screenshots/Recordings

Staking Before

stake_before.mov

Staking After

stake_after.mov

NFT Send Before

nft_send_before.mov

NFT Send After

nft_send_after.mov

Native Send Before

native_send_before.mov

Native Send After

native_send_after.mov

Send Before

send_before.mov

Send After

send_after.mov

Lending Icon Fix Before

lending_before.mov

Lending Icon Fix After

lending_after.mov

Before

~

After

~

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
  • I've tested with a power user scenario
  • I've instrumented key operations with Sentry traces for production performance metrics

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Navigation stack replacement affects multiple confirmation flows; lending adapter heuristics could mis-identify tokens on unusual receipt shapes, though fallbacks and tests cover main paths.

Overview
Fixes two Activity-redesign issues: post-transaction back navigation and lending deposit row/details display.

Post-tx navigation: Adds navigateToActivityAfterConfirmation, which uses root StackActions.replace for Activity when TRANSACTIONS_VIEW is on the root stack (Money-account on), instead of pushing Activity on top of a consumed confirmation. Falls back to plain navigate otherwise. Wired through useTransactionConfirm, useConfirmActions, Earn lending confirm views, and staking footer.

Lending token display: local-transaction now resolves lending deposit sourceToken from simulation balance decreases or user outgoing ERC-20 transfers (pool or aToken recipient), not txParams.to. New enrichTokenFromApi fills symbol/decimals from the tokens API for lending rows (useActivityListItemRowContent), avatars, and SwapDetails amount headers when adapters only supply atomic amounts + assetId.

Reviewed by Cursor Bugbot for commit 212f871. Bugbot is set up for automated code reviews on this repo. Configure here.

Lending deposit rows derived their token from the pool contract (txParams.to) instead of the underlying deposited asset, so the Activity row had no resolvable icon, symbol, or amount. Resolve the supplied token from simulation data / the outgoing Transfer log (falling back to the pool address), and enrich lending token metadata from the tokens API so the row shows the correct avatar and amount.

TMCU-1082
…ter a transaction

After a full-screen confirmation submitted, the redirect pushed the Activity screen on top of the now-consumed confirmation (approval deleted via deleteAfterResult), so pressing back returned to it and it rendered an infinite loader. A new navigateToActivityAfterConfirmation helper replaces the confirmation's stack with Activity in a single StackActions.replace, avoiding the stale screen and the pop+push double-animation. Wired into the shared transaction confirm hook, batch approvals, the Earn lending deposit/withdrawal views, and the legacy staking footer.

TMCU-1001
@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamask-ci metamask-ci Bot added the team-mobile-ux Mobile UX team label Jul 10, 2026
@metamask-ci

metamask-ci Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Pre-merge author checklist has unchecked items (e.g. "I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors."). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx 0/57 0/185 0/365
app/util/navigation/navigateToActivityAfterConfirmation.test.tsx 0/57 0/185 0/365

AI-detected flaky patterns

app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx

  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The 'display currency conversion' describe block restores specific mock return values in afterEach but never calls jest.clearAllMocks(). This means call counts for mockCurrency, mockConversionRate, and mockUsdConversionRate accumulate across every test in this block. If any future test in this file or a sibling describe block asserts toHaveBeenCalledTimes(N) on these selectors, it will see a higher count than expected and fail intermittently depending on test execution order. Additionally, if a test throws mid-body after calling mockCurrency.mockReturnValue('eur'), the afterEach still runs and resets the return value — but the call count from the failed test is never cleared, leaving stale state for subsequent tests. The outer 'row content' describe has jest.clearAllMocks() in beforeEach, but this inner describe does not inherit that guard.
    • Suggested fix in app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx:1:
      -describe('ActivityListItemRow — display currency conversion', () => {
      -  const mockCurrency = jest.mocked(selectCurrentCurrency);
      -  const mockConversionRate = jest.mocked(selectConversionRateByChainId);
      -  const mockUsdConversionRate = jest.mocked(selectUSDConversionRateByChainId);
      -  afterEach(() => {
      -    mockCurrency.mockReturnValue('usd');
      -    mockConversionRate.mockReturnValue(2500);
      -    mockUsdConversionRate.mockReturnValue(2500);
      -  });
      +describe('ActivityListItemRow — display currency conversion', () => {
      +  const mockCurrency = jest.mocked(selectCurrentCurrency);
      +  const mockConversionRate = jest.mocked(selectConversionRateByChainId);
      +  const mockUsdConversionRate = jest.mocked(selectUSDConversionRateByChainId);
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +    mockCurrency.mockReturnValue('usd');
      +    mockConversionRate.mockReturnValue(2500);
      +    mockUsdConversionRate.mockReturnValue(2500);
      +  });
      +  afterEach(() => {
      +    jest.clearAllMocks();
      +  });
  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The 'ERC-20 fiat address casing' describe block uses afterEach to restore only mockContractExchangeRates to a specific return value, but never calls jest.clearAllMocks(). Call counts for mockContractExchangeRates accumulate across tests in this block. If a test in this block sets mockContractExchangeRates.mockReturnValue(ratesFor(USDC_CHECKSUM)) and then a subsequent test (or a test in a later describe block that runs after this one) checks toHaveBeenCalledTimes, it will see inflated counts. The missing clearAllMocks also means that any other mocks touched inside these tests (e.g. getNetworkImageSource, useTokensData) retain their call history across tests in this block, creating order-dependent failures.
    • Suggested fix in app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx:1:
      -describe('ActivityListItemRow — ERC-20 fiat address casing (TMCU-937)', () => {
      -  const mockContractExchangeRates = jest.mocked(
      -    selectContractExchangeRatesByChainId,
      -  );
      -  const USDC_LOWER = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
      -  const USDC_CHECKSUM = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
      -  const ratesFor = (address: string) =>
      -    ({ [address]: { price: 0.0004 } }) as ReturnType<
      -      typeof selectContractExchangeRatesByChainId
      -    >;
      -  afterEach(() => {
      -    mockContractExchangeRates.mockReturnValue(ratesFor(LINEA_MUSD_ADDRESS));
      -  });
      +describe('ActivityListItemRow — ERC-20 fiat address casing (TMCU-937)', () => {
      +  const mockContractExchangeRates = jest.mocked(
      +    selectContractExchangeRatesByChainId,
      +  );
      +  const USDC_LOWER = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
      +  const USDC_CHECKSUM = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
      +  const ratesFor = (address: string) =>
      +    ({ [address]: { price: 0.0004 } }) as ReturnType<
      +      typeof selectContractExchangeRatesByChainId
      +    >;
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +    mockContractExchangeRates.mockReturnValue(ratesFor(LINEA_MUSD_ADDRESS));
      +  });
      +  afterEach(() => {
      +    jest.clearAllMocks();
      +  });

app/util/navigation/navigateToActivityAfterConfirmation.test.tsx

  • J1 — async callback without act() wrapping a state update (high)
    • In all three integration tests (flows 1, 2, 3), fireEvent.press triggers navigateToActivityAfterConfirmation, which calls dispatch(StackActions.replace(...)) or navigate(...) on the React Navigation state. Although fireEvent wraps the press in a synchronous act(), React Navigation's internal state reconciliation after a dispatch can schedule additional asynchronous React state updates (re-renders, context propagation). The assertions on focusedRouteName(ref) and rootRouteNames(ref) are made immediately after the press without an await act(async () => {}) guard, so they may read stale navigation state before the update has flushed — causing intermittent failures under load. The author already uses act(() => { ref.goBack(); }) for the subsequent back-navigation step, showing awareness of the pattern; the same guard is needed around the initial press. The same issue exists in flows 2 (line ~168) and 3 (line ~207).
    • Suggested fix in app/util/navigation/navigateToActivityAfterConfirmation.test.tsx:131:
      -    fireEvent.press(getByTestId('redirect-trigger'));
      -
      -    // Whole StakeScreens flow stack replaced (its nested build screen goes too).
      -    expect(focusedRouteName(ref)).toBe(Routes.TRANSACTIONS_VIEW);
      -    expect(rootRouteNames(ref)).toEqual([
      -      Routes.HOME_TABS,
      -      Routes.TRANSACTIONS_VIEW,
      -    ]);
      -
      -    // Back returns to Wallet home, not the consumed confirmation.
      -    act(() => {
      -      ref.goBack();
      -    });
      -    expect(focusedRouteName(ref)).toBe(Routes.HOME_TABS);
      +    await act(async () => {
      +      fireEvent.press(getByTestId('redirect-trigger'));
      +    });
      +
      +    // Whole StakeScreens flow stack replaced (its nested build screen goes too).
      +    expect(focusedRouteName(ref)).toBe(Routes.TRANSACTIONS_VIEW);
      +    expect(rootRouteNames(ref)).toEqual([
      +      Routes.HOME_TABS,
      +      Routes.TRANSACTIONS_VIEW,
      +    ]);
      +
      +    // Back returns to Wallet home, not the consumed confirmation.
      +    act(() => {
      +      ref.goBack();
      +    });
      +    expect(focusedRouteName(ref)).toBe(Routes.HOME_TABS);

This check is informational only and does not block merging.

@vinnyhoward vinnyhoward marked this pull request as ready for review July 13, 2026 15:24
@vinnyhoward vinnyhoward requested review from a team as code owners July 13, 2026 15:24
@vinnyhoward vinnyhoward changed the title fix(activity): stuck confirmation on back after tx + missing lending deposit token icon fix(activity): stuck confirmation on back after tx + missing lending deposit token icon cp-8.3.0 Jul 13, 2026
@github-actions github-actions Bot added the risk:low AI analysis: low risk label Jul 13, 2026
Comment thread app/util/navigation/navigateToActivityAfterConfirmation.ts Outdated
Comment thread app/util/activity-adapters/adapters/local-transaction.ts Outdated
…ot route

StackActions.replace(TRANSACTIONS_VIEW) only resolves when Activity is registered on the root navigator (Money-account enabled). When the flag is off, Activity lives under the tabs, so guard on the root routeNames and fall back to a plain navigate() to match the prior behavior. Adds unit coverage for the replace / navigate / no-parent branches. Addresses Cursor Bugbot review.

TMCU-1001
…token from logs

The post-confirmation receipt-log fallback matched the first outgoing ERC-20 Transfer sent from the user, which could pick an unrelated transfer (e.g. a gas-fee token) earlier in the log. Also require the recipient to be the pool (txParams.to) so it matches the actual deposit transfer. Addresses Cursor Bugbot review.

TMCU-1082
@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:low AI analysis: low risk labels Jul 13, 2026
Comment thread app/util/activity-adapters/adapters/local-transaction.ts
…mcu-1001-activity-confirmation-back-navigation
…transfer for lending deposit token

Requiring the Transfer recipient to equal the pool (txParams.to) missed the real deposit log for Aave V3, which sends the underlying from the user to the reserve aToken rather than to the pool address. Instead, prefer a transfer to the pool when present (to disambiguate from an unrelated outgoing transfer such as a gas-fee token) and otherwise fall back to the first outgoing transfer from the user. Addresses the follow-up Cursor Bugbot finding.

TMCU-1082
…y details screen

The Activity transaction details screen (SwapDetails template) rendered the lending deposit amount and its fiat total from the raw token without applying decimals (e.g. -10,000 / $9,991.95 instead of -0.01 / ~$0.01), because it never enriched the token's decimals the way the Activity list row does. Enrich the source/destination token decimals from the tokens API in SwapDetails so both the amount and the fiat total resolve correctly. Moves the shared enrichTokenFromApi helper into activity-list-helpers (re-exported from the package index) with direct unit tests, and adds SwapDetails coverage.

TMCU-1082
…lint:tsc findings

- Extract the nested ternaries for the primary token and avatar tokens in useActivityListItemRowContent into if/else statements (SonarCloud). - Remove the now-unused Routes imports from the Earn lending deposit/withdrawal views (SonarCloud). - Fix the lint:tsc error in the lending deposit receipt-log fallback by destructuring the topic elements instead of passing the whole topics array to the helper.
@github-actions github-actions Bot added size-XL and removed size-L labels Jul 13, 2026
@github-actions github-actions Bot added size-L and removed size-XL labels Jul 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeConfirmations, SmokeStake, SmokeSwap, SmokeWalletPlatform, SmokeMoney
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: high
  • AI Confidence: 88%
click to see 🤖 AI reasoning details

E2E Test Selection:

The PR introduces several interconnected changes:

  1. New navigation utility navigateToActivityAfterConfirmation: This replaces direct navigation.navigate(Routes.TRANSACTIONS_VIEW) calls across multiple confirmation flows. The new utility uses StackActions.replace when Activity is a root-stack screen (to fix an infinite loader bug in Money-account-enabled mode), falling back to plain navigate. This is a critical navigation change affecting post-confirmation routing.

  2. Affected confirmation flows:

    • useTransactionConfirm.ts: All full-screen transaction confirmations (send ETH, ERC-20, contract interactions, etc.)
    • useConfirmActions.ts: Batch transaction (EIP-7702) confirmations
    • EarnLendingDepositConfirmationView: Lending deposit confirmations
    • EarnLendingWithdrawalConfirmationView: Lending withdrawal confirmations
    • FooterButtonGroup.tsx (Staking): Stake/unstake confirmations
  3. Activity display improvements:

    • enrichTokenFromApi new helper enriches token data (symbol, decimals) from API for activity items
    • SwapDetails.tsx: Swap activity details now enrich token data from API
    • useActivityListItemRowContent.ts: Lending activity items now use enriched token data
    • local-transaction.ts: Better lending deposit source token detection using simulation data and tx logs

Tag selection rationale:

  • SmokeConfirmations: Navigation after confirmation is changed for all transaction types. Must verify the post-confirmation routing works correctly.
  • SmokeStake: Staking and lending confirmation navigation directly changed; lending activity display improved.
  • SmokeSwap: Swap confirmation navigation changed (via useTransactionConfirm); swap activity details (SwapDetails.tsx) now use enriched token data.
  • SmokeWalletPlatform: Activity/transaction history display is affected by token enrichment changes; the navigation fix affects how users land on the activity view.
  • SmokeMoney: Fiat/card flows that use confirmations are affected by the navigation change; per tag description, when selecting SmokeSwap for flows that execute swaps, also select SmokeMoney.

Per tag descriptions: SmokeSwap and SmokeStake both require SmokeConfirmations (already selected). SmokeMoney flows that execute swaps also require SmokeSwap and SmokeConfirmations (already selected).

Performance Test Selection:
The changes are focused on navigation routing after confirmations and activity list token display enrichment. These are correctness/bug-fix changes rather than performance-sensitive changes. No performance-critical paths (app launch, login, asset loading, onboarding) are affected. The new navigateToActivityAfterConfirmation utility adds minimal overhead (one getParent() call and a route name check). No performance test tags are warranted.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

@matthewwalsh0 matthewwalsh0 requested a review from jpuri July 14, 2026 14:53
Comment on lines +32 to +33
// `replace` only resolves when Activity is a root-stack screen (Money-account
// enabled — the case with the bug). Otherwise fall back to plain `navigate`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We can probably remove the feature-specific comment since this is a generic nav util.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good callout. I'll remove these in a follow up PR @Matt561

@vinnyhoward vinnyhoward added this pull request to the merge queue Jul 14, 2026
Merged via the queue into main with commit 6e619fd Jul 14, 2026
162 of 163 checks passed
@vinnyhoward vinnyhoward deleted the fix-tmcu-1001-activity-confirmation-back-navigation branch July 14, 2026 16:08
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 14, 2026
@metamask-ci metamask-ci Bot added the release-8.4.0 Issue or pull request that will be included in release 8.4.0 label Jul 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-8.4.0 Issue or pull request that will be included in release 8.4.0 risk:medium AI analysis: medium risk size-L team-mobile-ux Mobile UX team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants